fix(scanner): always surface cisco static-analysis coverage caveat (MCP-2399)#663
Merged
Merged
Conversation
The bundled cisco-mcp-scanner runs 'static --tools tools.json': it analyzes
the exported tool definitions with YARA + readiness rules and never probes the
live server endpoint or makes a network request. An is_safe/SAFE result therefore
reflects the tool definitions, not the server's runtime behavior — so a clean
Cisco result for a remote/URL server must not be over-trusted as live coverage.
Previously the only caveat ('no network request was made') was emitted by
sanitizeCiscoStdout solely when the upstream output happened to contain the
hardcoded deepwiki placeholder line. If a future cisco-ai-mcp-scanner release
changes or drops that placeholder, the caveat would silently vanish while the
static-only limitation remains.
Prepend a permanent coverage caveat to every Cisco execution log, independent
of the deepwiki string, leading the output so it survives MaxLogBytes truncation.
Still strip the deepwiki placeholder line when present. Update docs to frame the
limitation as a coverage caveat rather than a purely cosmetic note.
Resolves the residual coverage-honesty concern from gh #383 (cosmetic leak
already fixed in #528).
Related #383
Deploying mcpproxy-docs with
|
| Latest commit: |
0ba6236
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://182bfd9e.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2399-cisco-coverage.mcpproxy-docs.pages.dev |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved via Claude Code review (Codex out): cisco coverage caveat (MCP-2399/#383). Reviewer verified fix + tests + CI green; VERDICT ACCEPT.
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27501576546 --repo smart-mcp-proxy/mcpproxy-go
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves the residual coverage-honesty concern raised in MCP-2399 / gh #383: the
cisco-mcp-scannerreturns blanketis_safe:true/SAFEfor URL targets and makes no network request, which risks being over-trusted as live coverage.Forensic context (what was already true)
deepwikiserver_urlleak in the Cisco execution log — fixed in fix(scanner): strip hardcoded deepwiki URL from Cisco scanner raw stdout #528 (sanitizeCiscoStdout).static --tools tools.jsonand performs real YARA + readiness analysis of the exported tool definitions;parseCiscoScannerOutputreads realscan_resultsand emits findings. So removing it from coverage would be wrong — it would discard legitimate tool-poisoning detection.The residual gap this PR fixes
The only caveat ("no network request was made") was emitted solely when upstream stdout happened to contain the
deepwikiplaceholder. If a future cisco-ai-mcp-scanner release changes/drops that placeholder, the caveat silently vanishes while the static-only limitation remains.Change
deepwikistring, leading the output so it survivesMaxLogBytestruncation. The caveat states plainly: static analysis of tool definitions only, no live endpoint probe, no network request — sois_safereflects the tool definitions, not runtime behavior.deepwikiplaceholder line when present (unchanged behavior).docs/features/security-scanner-plugins.mdnote from "cosmetic" to an explicit coverage caveat.This is backend + docs only (
internal/+docs/), no frontend/API surface change. The displayed stdout is log-viewing only; findings are parsed from the report bytes, so the annotation does not affect parsed results (existingTestSetScannerLogs_*cover this).Tests (TDD)
TestSanitizeCiscoStdout_SurfacesCoverageCaveatWithoutDeepwiki,TestSanitizeCiscoStdout_CaveatAlwaysLeadsOutputgo test ./internal/security/... -racegreen;golangci-lint(v2 CI config) 0 issues;go build ./cmd/mcpproxyclean.Disposition for gh #383
Already CLOSED (cosmetic fix shipped in #528). This PR closes out the broader coverage-caveat ask from MCP-2399.
Related #383